add change store extension for per-URI request claims#152
Merged
Conversation
This was referenced May 7, 2026
sbalabanov
requested changes
May 7, 2026
github-actions
Bot
force-pushed
the
preetam/changestore-extension
branch
from
1f6589e to
6057c35
Compare
Introduce a dedicated extension that tracks (URI, request_id) claims with mutable metadata, scoped to queue. Foundation for cross-request URI overlap detection; no consumers in this commit. - entity/change_record.go: immutable identity (URI, RequestID), mutable metadata - extension/changestore/: ChangeStore interface, mysql impl, mock, schema - INSERT IGNORE on writes for retry idempotency on (uri, request_id) - FindOverlapping is single-table; callers do their own liveness check - integration test under test/integration/extension/changestore/mysql/ - e2e suite applies the change schema (harmless no-op until consumers land)
The e2e suite_test applies the change schema, but the bazel test target was missing //extension/changestore/mysql/schema in its data list, so runfiles didn't ship the .sql file and ApplySchema couldn't find it.
… API
- schema: PRIMARY KEY (queue, uri, request_id) — queue-scoped lookups become
PK-prefix scans and the table is shardable by queue. Comment in the schema
explains why request_id stays in the PK (concurrent claims by different
requests coexist as distinct rows; same-request retries collide on the PK).
- schema: metadata JSON NOT NULL. The mysql impl writes '{}' for empty
metadata so callers don't need to know about the column constraint.
- interface: drop excludeRequestID from FindOverlapping. Callers that want to
skip self filter the result by RequestID themselves. Documented Create's
batch atomicity (single multi-row INSERT, all-or-nothing).
- mysql: FindOverlapping query now leads with `WHERE queue = ?` to align with
the new PK order.
- entity: expanded RequestID/Queue field comments to explain their PK roles.
- README: documents the new key shape and metadata semantics.
- integration tests: drop the 4th arg, replace TestFindOverlapping_ExcludesSelf
with a test that asserts the store does NOT exclude self, and add an
assertion that empty metadata round-trips as '{}'.
behinddwalls
force-pushed
the
preetam/changestore-extension
branch
from
6057c35 to
ff53ce6
Compare
The previous interface assumed a SQL-style backend that gives batch atomicity and multi-key WHERE IN queries for free, which over-constrains alternatives like DynamoDB or Bigtable. Move to single-record, single-URI primitives that any backend can implement cheaply; callers loop when they have multiple URIs (typically 1-5 per request). - Create(record ChangeRecord) — was Create([]ChangeRecord). No batch atomicity contract; same-PK conflict is INSERT IGNORE on the mysql impl, callers retry whole loops to converge. - GetByURI(queue, uri) — was FindOverlapping(queue, []uris). Returns every ChangeRecord for the (queue, uri) pair; caller loops over its URIs and filters by RequestID for self-exclusion. - README and integration tests updated for the new shape.
Captures the design lesson from PR #152: interfaces should be shaped for the technology space (SQL, KV, document, queues, RPC, …), not for the one implementation we're adding. Lists common over-constraints to avoid (batch atomicity, multi-key queries, server-side filters, cross-entity transactions, strict ordering / exactly-once, sync low-latency assumptions) and proposes the test: 'could DynamoDB / Kafka / Bigtable / a remote RPC / an in-memory map satisfy this signature without contortion?'
Mirrors the request-store pattern (RequestStore.UpdateState takes oldVersion + newVersion). The schema gains 'version INT NOT NULL'; ChangeRecord gains 'Version int32'; mysql impl writes/reads it. No UpdateMetadata method yet — added when the first metadata-enrichment writer arrives.
sbalabanov
approved these changes
May 20, 2026
behinddwalls
added a commit
that referenced
this pull request
May 21, 2026
## Summary Stacked on top of #152 (change store extension). Detects duplicate land requests by reading per-URI claims from the new change store. The `start` controller writes a claim row for each URI on the request; `validate` reads the change store and rejects requests whose URIs overlap with another in-flight request in the same queue. - **start.go** — after persisting the request, claim each URI in the change store. `INSERT IGNORE` makes this idempotent on queue redelivery (same `request_id` retry is a no-op). - **validate.go** — query the change store for any row with an overlapping URI in the same queue (excluding self). For each unique candidate `request_id` returned, look up the request and skip terminal/orphan owners. If any live owner remains, reject as a user error naming the conflicting `request_id`. - Two single-table queries per check, no cross-store SQL joins. - Stop wrapping every storage error as `Retryable`; default is plain `fmt.Errorf` per the `core/errs` framework + #134. - Wires `changeStore` into both controllers in `example/server/orchestrator/main.go`. ## Behavior - **Overlap (not full-set equality):** the change store returns rows for any URI in the new request that matches a row from another in-flight request, so partial-overlap is rejected — addresses @sbalabanov's review comment. - **Liveness:** terminal-state owners and orphans (whose request row is gone) are skipped, so stale claims don't block new requests indefinitely. - **Retries:** `start` tolerates `ErrAlreadyExists` from request creation as a same-id replay; the change store's `INSERT IGNORE` is idempotent for the same reason. ## Test Plan - Unit tests for `start` and `validate` cover happy path, overlap-with-live, overlap-with-terminal-skipped, overlap-with-orphan-skipped, multi-URI-same-owner deduped, and infra-error propagation. - Existing integration tests on `extension/storage/mysql` continue to pass after `GetActiveByQueue` removal. - Change store integration test (in #152) exercises the MySQL impl against a real DB. ## Issues --------- Co-authored-by: Preetam Dwivedi <preetam@uber.com> Co-authored-by: Preetam Dwivedi <behinddwalls@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Introduce a dedicated extension that tracks (URI, request_id) claims with mutable
metadata, scoped to queue. Foundation for cross-request URI overlap detection;
no consumers in this commit.
Test Plan
Issues
Stack